Skip to content

Conversation

@TheCodeTraveler
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler commented Jan 2, 2026

Description of Change

This PR implements [AttachedBindableProperty<T>("PropertyName")]:

The attribute can be applied to a Class, like so:

using CommunityToolkit.Maui;
using Microsoft.Maui.Controls;

namespace TestNamespace

[AttachedBindableProperty<string>("Text", // Name of the Attached Property
			      IsNullable = true, // Should generate a nullable type for T
			      DefaultValue = "Hello World", // The default value for the property
			      DefaultBindingMode = default, // The BindingMode to use on SetBinding() if no BindingMode is given. Default is <see cref="Microsoft.Maui.Controls.BindableProperty.Default"/>
			      ValidateValueMethodName = null, // Method name for <see cref="Microsoft.Maui.Controls.ValidateValueDelegate"/> to be run when a value is set. Default value is null
			      PropertyChangedMethodName = null, // Method name for <see cref="Microsoft.Maui.Controls.BindingPropertyChangedDelegate"/> to be run when a value has changed. Default value is null
			      PropertyChangingMethodName = null, // Method name for <see cref="Microsoft.Maui.Controls.BindingPropertyChangingDelegate"/> to be run when a value is set. Default value is null
			      CoerceValueMethodName = null, // Method name for <see cref="Microsoft.Maui.Controls.CoerceValueDelegate"/> used to coerce the range of a value. Default value is null
			      DefaultValueCreatorMethodName = null, // Method name for <see cref="Microsoft.Maui.Controls.CreateDefaultValueDelegate"/> used to initialize default value for reference types. Default value is null
			      BindablePropertyXmlDocumentation = null, // Custom XML Comments added to BindableProperty
			      GetterMethodXmlDocumentation = null, // Custom XML Comments added to Get{PropertyName}(BindableProperty bindable)
			      SetterMethodXmlDocumentation = null, // Custom XML Comments added to <c>Set{PropertyName}(BindableProperty bindable, T value)
			      BindablePropertyAccessibility = AccessModifier.Public, // The access modifier applied to the generated field <see cref="Microsoft.Maui.Controls.BindableProperty"/>
			      GetterAccessibility = AccessModifier.Public, // The access modifier applied to the generated method Get{PropertyName}(BindableProperty bindable)
			      SetterAccessibility = AccessModifier.Public)] // The access modifier applied to the generated method Set{PropertyName}(BindableProperty bindable, T value)
public partial class TestClass : View
{
    // Alternatively, it can be placed on the constructor, for example:
    // [AttachedBindableProperty<string>("Text")]
    public TestClass()
    {
    }
}

Both usages will generate the following source code:

// <auto-generated>
// See: CommunityToolkit.Maui.SourceGenerators.Internal.AttachedBindablePropertyAttributeSourceGenerator
#pragma warning disable
#nullable enable
namespace TestNamespace;
public partial class TestClass
{
	/// <summary>
	/// Attached BindableProperty for the Text property.
	/// </summary>
	public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.CreateAttached("Text", typeof(string), typeof(TestNamespace. TestClass),(string?)"Hello World", (Microsoft.Maui.Controls.BindingMode)0, null, null, null, null, null);
	/// <summary>
	/// Gets Text for the <paramref = "bindable"/> child element.
	/// </summary>
	public static string? GetText(global::Microsoft.Maui.Controls.BindableObject bindable) => (string)bindable.GetValue(TextProperty);
	/// <summary>
	/// Sets Text for the <paramref = "bindable"/> child element.
	/// </summary>
	public static void SetText(global::Microsoft.Maui.Controls.BindableObject bindable, string? value) => bindable.SetValue(TextProperty, value);
}

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard

Additional information

This PR is still in draft as some unit tests are still failing and need to be resolved.

This PR unblocks #3016 and #3000

Copy link
Member

@pictos pictos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, I've a couple of comments about it

var attachedBindablePropertiesCount = 0;
foreach (var value in values)
{
foreach (var abp in value.BindableProperties.AsImmutableArray())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the usage of AsImmutableArray?

{
var newBuffer = System.Buffers.ArrayPool<AttachedBindablePropertyModel>.Shared.Rent(attachedBindablePropertiesBuffer.Length * 2);
Array.Copy(attachedBindablePropertiesBuffer, newBuffer, attachedBindablePropertiesBuffer.Length);
System.Buffers.ArrayPool<AttachedBindablePropertyModel>.Shared.Return(attachedBindablePropertiesBuffer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't cleaning up the array when returning to the pool, this will make this array to hold values there, which can lead in bugs if we use the full Length of the array instead of the right number of BindableProperties

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what's missing. What do you recommend for the fix?

@TheCodeTraveler
Copy link
Collaborator Author

Looks good to go. Very well done. The tests look very comprehensive and complete.

Thanks @ne0rrmatrix! Could you give it one more review? I've just added support for BindableProperty.CreateAttachedReadOnly() in this commit: 8294bf8

ne0rrmatrix
ne0rrmatrix previously approved these changes Jan 11, 2026
@stephenquan
Copy link
Contributor

stephenquan commented Jan 11, 2026

Some alternate syntaxes we can consider:

Option 1: Attach the new attribute to dummy properties (alternatively, consider a static or const)

  • Pro: a very simple syntax
  • Con: creates bloated unused properties
  • Con: issue with XML docs
public partial class TestClass
{
	// Attached the new attribute to a dummy property
	[AttachedBindableProperty]
	public partial string? Text { get; set; } = "Hello World";
}

Proposed generated source code for option 1:

// Use the dummy property to extract type and name and generate the attached property
public partial class TestClass
{
	public partial string? Text
	{
		get => field;
		set => field = value;
	}
	/// <summary>
	/// Attached BindableProperty for the Text property.
	/// </summary>
	public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.CreateAttached("Text", typeof(string), typeof(TestNamespace. TestClass),(string?)"Hello World", (Microsoft.Maui.Controls.BindingMode)0, null, null, null, null, null);
	/// <summary>
	/// Gets Text for the <paramref = "bindable"/> child element.
	/// </summary>
	public static string? GetText(global::Microsoft.Maui.Controls.BindableObject bindable) => (string)bindable.GetValue(TextProperty);
	/// <summary>
	/// Sets Text for the <paramref = "bindable"/> child element.
	/// </summary>
	public static void SetText(global::Microsoft.Maui.Controls.BindableObject bindable, string? value) => bindable.SetValue(TextProperty, value);
}

Option 2: Attach the new attribute to the real getter/setter, e.g.

  • Pro: Developers has to provide their GetText/SetText in this style anyway
  • Pro: Developers don't need to remember how to write the attached property
  • Pro: Has a better XML doc integration
  • Pro: Potentially addresses the readonly scenario because that will be the difference of one method versus two
public partial class TestClass
{
	[AttachedBindableProperty]
	public static string? GetText(BindableObject view)
	{
		return (string?)view.GetValue(TextProperty);
	}
	[AttachedBindableProperty]
	public static void SetText(BindableObject view, string? value)
	{
		view.SetValue(TextProperty, value);
	}
}

Proposed generated source code for option 2:

// Generate the attached property
public partial class TestClass
{
    /// <summary>
    /// Attached BindableProperty for the Text property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.CreateAttached("Text", typeof(string), typeof(TestNamespace. TestClass),(string?)"Hello World", (Microsoft.Maui.Controls.BindingMode)0, null, null, null, null, null);}

@pictos
Copy link
Member

pictos commented Jan 11, 2026

@stephenquan, thanks for your suggestions... But I've to say that I don't like that design.

  1. It will create two methods (getter and setter) on the assembly, it will increase the app size in an order of O(n) for nothing. Also, it will create a property that will confuse devs, they may think they can get/set the value from that property and will fail.
  2. For the method, I'm not sure about it, the idea of the SG is to avoid that boilerplate, the methods also are a boilerplate since they always will be the same.

But, looking at your suggestions for the methods gave me an idea on something that's missing. Right now, the SG doesn't provide an easy way for devs to insert behavior on the generated methods.

@TheCodeTraveler I think we should add it, and we can use the MVVM toolkit as a reference, it uses partial methods to allow devs to add behavior. I think we should do the same. I ain't 100% sure about the API design, but the general idea would be

// SG Code
	public static void SetText(BindableObject view, string? value)
	{
                BeforeSet(view, ref value);
		view.SetValue(TextProperty, value);
                AfterSet(view, value);
	}
        partial void BeforeSet(BindableObject view, ref string? value);
        partial void AfterSet(BindableObject view, string? value);

// User code
partial void BeforeSet(BindableObject view, string? value)
{
     if (!decimal.TryParse(value, out var money))
            value = string.Empty;
    
     value = $"R${money}";
}

@bijington
Copy link
Contributor

@pictos that looks good. Re: naming should we follow the Changing (BeforeSet) and Changed (AfterSet) approach? We could even pass in the new value to changing if we think it helps

@TheCodeTraveler
Copy link
Collaborator Author

TheCodeTraveler commented Jan 11, 2026

Love the idea of adding partial methods! But, does it differ enough from the existing void PropertyChanged(Bindable, object, object) and void PropertyChanging(Bindable,object,object) delegates in BindableProperty that devs can hook into? I think we'd just be this existing functionality, no?

@pictos
Copy link
Member

pictos commented Jan 11, 2026

@TheCodeTraveler oh yeah, I forgot those, yeah so I believe it's good to go

@stephenquan
Copy link
Contributor

stephenquan commented Jan 11, 2026

I had another crack at this and came up with more syntaxes to consider:

Option 3 user stubs out delegates:

public partial class TestClass
{
    [AttachedBindableProperty]
    public static partial Func<BindableObject, string?> GetText { get; }
    [AttachedBindableProperty]
    public static partial Action<BindableObject, string?> SetText { get; }
}

SG produces:

public partial class TestClass
{
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.CreateAttached("Text", typeof(string), typeof(TestNamespace.TestClass), (string?)"Hello World", (Microsoft.Maui.Controls.BindingMode)0, null, null, null, null, null);
    public static partial Func<BindableObject, string?> GetText { get => field; } = new((bindable) => (string)bindable.GetValue(TextProperty));
    public static partial Action<BindableObject, string?> SetText { get => field; } = new((bindable, value) => bindable.SetValue(TextProperty, value));
}

Option 4 (Option 3 refactored with easier-to-remember delegates)

public partial class TestClass
{
    [AttachedBindableProperty]
    public static partial AttachedBindablePropertyGetter<string?> GetText { get; }
    [AttachedBindableProperty]
    public static partial AttachedBindablePropertySetter<string?> SetText { get; }
}

SG produces:

public partial class TestClass
{
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.CreateAttached("Text", typeof(string), typeof(TestClass), (string?)default);
    public static partial AttachedBindablePropertyGetter<string?> GetText { get => field; } = new((BindableObject view) =>
    {
        return (string?)view.GetValue(TextProperty);
    });
    public static partial AttachedBindablePropertySetter<string?> SetText { get => field; } = new((bindable, value) =>
    {
        bindable.SetValue(TextProperty, value);
    });
}

We define the helpers as delegates, i.e.

public delegate T AttachedBindablePropertyGetter<T>(BindableObject view);
public delegate void AttachedBindablePropertySetter<T>(BindableObject view, T value);

pictos
pictos previously approved these changes Jan 15, 2026
@TheCodeTraveler TheCodeTraveler dismissed stale reviews from ne0rrmatrix and pictos via ed177c2 January 15, 2026 22:35
@TheCodeTraveler TheCodeTraveler merged commit c1d2ee3 into main Jan 15, 2026
10 checks passed
@TheCodeTraveler TheCodeTraveler deleted the Add-`-AttachedBindableProperty]` branch January 15, 2026 22:46
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants